Skip to content

Conversation

@cmp0xff
Copy link
Contributor

@cmp0xff cmp0xff commented Nov 20, 2025

@cmp0xff cmp0xff requested a review from rhshadrach as a code owner November 20, 2025 09:17
Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change you Series.value_counts docstring too and add a test - something like

ser = pd.Series(np.random.randint(0, 100, 10000))
result = ser.value_counts()
expected = ser.value_counts(sort=False).sort_values(ascending=False, kind="stable")
tm.assert_series_equal(result, expected)

seems to fairly reliably fail without this PR (100000 out of 100000 times). Should set the NumPy random seed in the test.

I don't think this is a breaking change since unstable sorting doesn't guarantee anything about the output order. Would also like another eye on this.

@cmp0xff
Copy link
Contributor Author

cmp0xff commented Nov 23, 2025

Change you Series.value_counts docstring too

Series inherits value_counts from IndexOpsMixin, and I've fixed the docstring there.

and add a test

Thank you for the hint, dc431ca

I don't think this is a breaking change since unstable sorting doesn't guarantee anything about the output order. Would also like another eye on this.

Do I need to react somehow?

@cmp0xff cmp0xff requested a review from rhshadrach November 23, 2025 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ENH: make sorting algorithm options to value_counts and record it in the docs

2 participants